Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't throw in _transform of BulkLoad #1590

Merged
merged 6 commits into from
Aug 7, 2024

Conversation

everhardt
Copy link
Contributor

I got an uncaughtException, caused by trying to write a very large (negative) number using BulkLoad to a decimal column. Seems similar to #421. The stack trace I got:

RangeError: The value of "value" is out of range. It must be >= 0 and <= 4294967295. Received 3_.40_282_346_638_528_86e_+42
  File "node:internal/errors", line 496, in __node_internal_captureLargerStackTrace
  File "node:internal/errors", line 405, in new NodeError
  File "node:internal/buffer", line 74, in checkInt
  File "node:internal/buffer", line 694, in writeU_Int32LE
  File "node:internal/buffer", line 707, in Buffer.writeUInt32LE
  File "/app/node_modules/tedious/src/data-types/decimal.ts", line 77, in Object.generateParameterData
  File "<anonymous>", line unknown, in generateParameterData.next
  File "/app/node_modules/tedious/src/bulk-load.ts", line 209, in _transform
  File "node:internal/streams/transform", line 175, in Transform._write
  File "node:internal/streams/writable", line 392, in writeOrBuffer
  File "node:internal/streams/writable", line 333, in _write
  File "node:internal/streams/writable", line 337, in Writable.write
  File "node:internal/streams/readable", line 777, in Readable.ondata
  File "node:events", line 517, in Readable.emit
  File "node:internal/streams/readable", line 550, in Readable.read
  File "node:internal/streams/readable", line 1032, in flow
  File "node:internal/streams/readable", line 1013, in resume_
  File "node:internal/process/task_queues", line 82, in process.processTicksAndRejections

bulk-load:209 is inside the _tranform function, for which I think the following in the nodejs documentation holds:

Throwing an Error from within these methods or manually emitting an 'error' event results in undefined behavior.

Therefore, I propose to catch such errors and call the callback with it. I verified that solved my case.

@arthurschreiber
Copy link
Collaborator

Can you add tests for this change?

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.29%. Comparing base (42617e4) to head (b6b88f1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1590      +/-   ##
==========================================
- Coverage   79.01%   78.29%   -0.73%     
==========================================
  Files          93       93              
  Lines        4876     4878       +2     
  Branches      937      937              
==========================================
- Hits         3853     3819      -34     
- Misses        720      761      +41     
+ Partials      303      298       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@everhardt
Copy link
Contributor Author

Can you add tests for this change?

Done!

@MichaelSun90 MichaelSun90 merged commit afd3b54 into tediousjs:master Aug 7, 2024
26 of 27 checks passed
Copy link

github-actions bot commented Aug 7, 2024

🎉 This PR is included in version 18.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants